fix(core): accept absolute paths in @file input; suppress proxy warning in CI by EvanYao826 · Pull Request #907 · larksuite/cli

EvanYao826 · 2026-05-15T08:42:18Z

Summary

Fixes #811

Two agent-friendly UX improvements:

1. Accept absolute paths in `@file` input

ReadInputFile now accepts absolute paths after safety validation (control characters, symlink resolution). This allows AI agents and scripts to use /tmp/ and other absolute paths with --markdown @/tmp/content.md and similar @file syntax.

For relative paths, behavior is unchanged (FileIO restricts to cwd).

2. Suppress proxy warning in CI

WarnIfProxied is now suppressed when:

LARK_CLI_NO_PROXY_WARNING env var is set (any non-empty value)
Running in CI environments (CI env var is set)

This prevents the proxy warning from dominating output in agent loops and scripts that make many successive lark-cli calls.

Changes

internal/cmdutil/resolve.go: ReadInputFile uses os.ReadFile with SafeAbsoluteInputPath for absolute paths
internal/util/proxy.go: WarnIfProxied checks LARK_CLI_NO_PROXY_WARNING and CI env vars

Summary by CodeRabbit

Improvements
- Enhanced file input handling to safely accept and validate absolute file paths with clearer error mapping.
- Added validation utilities for absolute paths to better detect unsafe paths and resolve symlinks.
- Proxy warning suppression now respects environment controls and auto-detects CI; warnings still redact URL credentials.

…ng in CI Two agent-friendly UX improvements: 1. ReadInputFile now accepts absolute paths after safety validation (control characters, symlink resolution). This allows AI agents and scripts to use /tmp/ and other absolute paths with --markdown @path. 2. WarnIfProxied is now suppressed when LARK_CLI_NO_PROXY_WARNING is set or when running in CI environments (CI env var). This prevents the warning from dominating output in agent loops and scripts. Fixes larksuite#811

coderabbitai · 2026-05-15T08:42:32Z

📝 Walkthrough

Walkthrough

Accept absolute file paths for input flags by validating them with localfileio.SafeAbsoluteInputPath and reading validated absolute paths with vfs.ReadFile. Suppress the one-time proxy warning when LARK_CLI_NO_PROXY_WARNING is set or when running in CI (CI non-empty).

Changes

UX improvements for agents and CI

Layer / File(s)	Summary
Absolute path file input handling `internal/cmdutil/resolve.go`, `internal/validate/path.go`, `internal/vfs/localfileio/path.go`	`ReadInputFile` checks `filepath.IsAbs(path)`, validates absolute paths via `localfileio.SafeAbsoluteInputPath`, reads validated absolute files with `vfs.ReadFile`, and wraps errors with `wrapInputFileError`. Adds `SafeAbsoluteInputPath` validation wrapper in `internal/validate` and new absolute-path helpers in `internal/vfs/localfileio`.
Proxy warning suppression in CI and with env var `internal/util/proxy.go`	`WarnIfProxied` skips emitting the one-time proxy warning when `LARK_CLI_NO_PROXY_WARNING` is set or when `CI` is non-empty; retains `LARK_CLI_NO_PROXY` behavior and redaction of proxy credentials.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

larksuite/cli#247: Overlaps changes to internal/util/proxy.go and proxy warning behavior.

Suggested labels

size/L

Suggested reviewers

liangshuo-1

Poem

🐰 I nibble paths both near and far,
Absolute or relative, I know what they are.
Warnings tucked quiet when CI takes the stage,
Agents can whisper and scripts turn the page.
hops off with a carrot and a smile

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes both main changes in the PR: accepting absolute paths in `@file` input and suppressing proxy warnings in CI environments.
Description check	✅ Passed	The PR description covers the Summary section, explains both changes clearly, and documents file modifications, though it doesn't include explicit Test Plan or Related Issues sections from the template.
Linked Issues check	✅ Passed	The code changes successfully address both objectives from issue `#811`: absolute paths are now accepted after safety validation [internal/cmdutil/resolve.go, internal/vfs/localfileio/path.go], and proxy warnings are suppressed via LARK_CLI_NO_PROXY_WARNING and CI env var checks [internal/util/proxy.go].
Out of Scope Changes check	✅ Passed	All changes in the PR directly address the two objectives from issue `#811`. Changes to validate/path.go serve as a helper for absolute-path validation, and all modifications are focused on the stated goals.
Docstring Coverage	✅ Passed	Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/cmdutil/resolve.go`:
- Around line 87-99: The absolute-path branch calls a non-existent
localfileio.SafeAbsoluteInputPath and uses os.ReadFile directly: either
implement a new SafeAbsoluteInputPath in the localfileio package (perform the
same control-char checks, symlink resolution and safety rules as SafeInputPath
but allowing absolute paths) or change the code to reject absolute paths and
route callers to the existing SafeInputPath; once decided, replace the
os.ReadFile(safePath) call with vfs.ReadFile(safePath) and preserve the existing
error handling via wrapInputFileError so filesystem access remains mockable
(refer to filepath.IsAbs, localfileio.SafeInputPath/SafeEnvDirPath,
localfileio.SafeAbsoluteInputPath, vfs.ReadFile, fileIO.Open, and
wrapInputFileError).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6bdc0dd5-c4b8-4bc5-ad19-70b97e957e9e

📥 Commits

Reviewing files that changed from the base of the PR and between 4a45e00 and 522c06f.

📒 Files selected for processing (2)

internal/cmdutil/resolve.go
internal/util/proxy.go

EvanYao826 · 2026-05-15T09:07:29Z

@CLAassistant recheck please

EvanYao826 · 2026-05-15T09:27:43Z

@CLAassistant recheck

Address CodeRabbit review: - Add SafeAbsoluteInputPath to localfileio and validate packages - Replace os.ReadFile with vfs.ReadFile to preserve test mocking - Remove unused os import

CLAassistant · 2026-05-15T09:42:43Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/validate/path.go (1)
31-37: ⚡ Quick win

Return the validated/resolved path instead of dropping it.

This wrapper discards the canonicalized path returned by localfileio.SafeAbsoluteInputPath, so callers (e.g. ReadInputFile in internal/cmdutil/resolve.go) have nothing to use except the original raw path for the subsequent vfs.ReadFile. That defeats the symlink-resolution step (it widens the TOCTOU window between validate and read) and is also inconsistent with the sibling SafeInputPath / SafeOutputPath wrappers above, which both return (string, error). Prefer returning the resolved path and have the caller read via that value.
♻️ Proposed change
-// SafeAbsoluteInputPath validates an absolute path for safety (control
-// characters, symlink resolution) without restricting to the working directory.
-func SafeAbsoluteInputPath(path string) error {
-	_, err := localfileio.SafeAbsoluteInputPath(path)
-	return err
-}
+// SafeAbsoluteInputPath validates an absolute path for safety (control
+// characters, symlink resolution) without restricting to the working directory.
+// Returns the canonicalized path; callers should read via the returned value
+// to avoid TOCTOU between validation and read.
+func SafeAbsoluteInputPath(path string) (string, error) {
+	return localfileio.SafeAbsoluteInputPath(path)
+}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/validate/path.go` around lines 31 - 37, The wrapper
SafeAbsoluteInputPath currently discards the canonicalized path returned by
localfileio.SafeAbsoluteInputPath; change its signature to return (string,
error) and return the resolved path and error so callers (e.g. ReadInputFile in
internal/cmdutil/resolve.go) can use the canonical path for subsequent reads;
follow the pattern used by SafeInputPath and SafeOutputPath by returning the
resolved string from SafeAbsoluteInputPath rather than only the error.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/vfs/localfileio/path.go`:
- Around line 103-126: safePathAbsolute / SafeAbsoluteInputPath claim to
validate absolute paths but never enforce absoluteness; add a check using
filepath.IsAbs at the start of safePathAbsolute (or in SafeAbsoluteInputPath
before delegating) and return a clear error if the input is not absolute so
callers (e.g., ReadInputFile) can't pass relative paths like "../../etc/passwd";
place the check after RejectControlChars and before
filepath.Clean/resolveNearestAncestor, and reference the functions
safePathAbsolute and SafeAbsoluteInputPath and the stdlib function
filepath.IsAbs when making the change.

---

Nitpick comments:
In `@internal/validate/path.go`:
- Around line 31-37: The wrapper SafeAbsoluteInputPath currently discards the
canonicalized path returned by localfileio.SafeAbsoluteInputPath; change its
signature to return (string, error) and return the resolved path and error so
callers (e.g. ReadInputFile in internal/cmdutil/resolve.go) can use the
canonical path for subsequent reads; follow the pattern used by SafeInputPath
and SafeOutputPath by returning the resolved string from SafeAbsoluteInputPath
rather than only the error.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7c7a6678-697e-473d-9f26-8ea2cdfcae29

📥 Commits

Reviewing files that changed from the base of the PR and between 522c06f and c66448e.

📒 Files selected for processing (3)

internal/cmdutil/resolve.go
internal/validate/path.go
internal/vfs/localfileio/path.go

🚧 Files skipped from review as they are similar to previous changes (1)

internal/cmdutil/resolve.go

EvanYao826 · 2026-05-15T09:45:53Z

@CLAassistant recheck

github-actions Bot added the size/M Single-domain feat or fix with limited business impact label May 15, 2026

coderabbitai Bot reviewed May 15, 2026

View reviewed changes

Comment thread internal/cmdutil/resolve.go

fix: add SafeAbsoluteInputPath and use vfs.ReadFile

c66448e

Address CodeRabbit review: - Add SafeAbsoluteInputPath to localfileio and validate packages - Replace os.ReadFile with vfs.ReadFile to preserve test mocking - Remove unused os import

coderabbitai Bot reviewed May 15, 2026

View reviewed changes

Comment thread internal/vfs/localfileio/path.go

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core): accept absolute paths in @file input; suppress proxy warning in CI#907

fix(core): accept absolute paths in @file input; suppress proxy warning in CI#907
EvanYao826 wants to merge 2 commits into
larksuite:mainfrom
EvanYao826:fix/agent-friendly-ux

EvanYao826 commented May 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 15, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

EvanYao826 commented May 15, 2026

Uh oh!

EvanYao826 commented May 15, 2026

Uh oh!

CLAassistant commented May 15, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

EvanYao826 commented May 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

EvanYao826 commented May 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. Accept absolute paths in @file input

2. Suppress proxy warning in CI

Changes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

EvanYao826 commented May 15, 2026

Uh oh!

EvanYao826 commented May 15, 2026

Uh oh!

CLAassistant commented May 15, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

EvanYao826 commented May 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

EvanYao826 commented May 15, 2026 •

edited by coderabbitai Bot

Loading

1. Accept absolute paths in `@file` input

coderabbitai Bot commented May 15, 2026 •

edited

Loading